Common: allow access to the reference comparator plots in derived classes #2578
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aferrero2707
requested review from
Barthelemy,
justonedev1 and
knopers8
as code owners
aferrero2707
force-pushed
the
common-expose-refcomp-plots
branch
from
5c9975f to
9ba37f8
Compare
Contributor
Author
|
@Barthelemy @knopers8 @justonedev1 this is another PR related to the reference comparator task. I am preparing another PR for the MCH code that will make use of the new functionality introduced here, that is to compare some plots with their references within the post-processing task that creates the plots, without having to run a dedicated ReferenceComparatorTask (and without extra traffic to the QCDB). |
Barthelemy
approved these changes
Jul 3, 2025
Collaborator
Barthelemy
left a comment
Barthelemy
left a comment
There was a problem hiding this comment.
Good idea !
I have just added a couple of trivial comments.
| static TH1* getRatioPlotFromCanvas(TCanvas* canvas) | ||
| { | ||
| // Get the pad containing the current histogram, as well as the reference one in the case of 1-D plots | ||
| TPad* padHistRatio = (TPad*)canvas->GetPrimitive(TString::Format("%s_PadHistRatio", canvas->GetName())); |
Collaborator
There was a problem hiding this comment.
why not a dynamic_cast ?
| static std::pair<TH1*, TH1*> getPlotsFromCanvas(TCanvas* canvas, std::string& message) | ||
| { | ||
| // Get the pad containing the current histogram, as well as the reference one in the case of 1-D plots | ||
| TPad* padHist = (TPad*)canvas->GetPrimitive(TString::Format("%s_PadHist", canvas->GetName())); |
Collaborator
There was a problem hiding this comment.
why not a dynamic_cast ?
Contributor
Author
There was a problem hiding this comment.
@Barthelemy indeed, a dynamic cast is better.
Regarding the ReferenceUtils.h, is there a specific reason why the getReferencePlot() function was implemented as static? I did the same for the other two functions added here, but I am not 100% sure this is what you had in mind...
…asses The changes allow to use the ReferenceComparatorTask as a base class, providing access to the reference comparison plots for the derived classes. A typical use case would be a post-processing task that produces some derived plots, which also need to compared with some reference. With the current production code, this involves adding a dedicated ReferenceComparatorTask in the workflow, that fetches the plots from the post-processing task from the QCDB. This commit allows to solve the above problem by deriving the post-processing task from the ReferenceComparatorTask, such that the reference comparison plots can be filled directly by the derived task, without going through the QCDB. This is achieved with the following two changes: * addition of a function that exposes the reference comparison plots in the ReferenceComparatorTask interface * storage of the pointer to the reference histogram in the ReferenceComparatorPlot object, such that it does not need to be passed to the `update()` method
Add to the ReferenceUtils.h header file the functions that allow to access the current, reference and ratio plots from the TCanvas created by the ReferenceComparartorPlot class. This will allow for example custom checkers to access the plots.
aferrero2707
force-pushed
the
common-expose-refcomp-plots
branch
from
9ba37f8 to
e40c600
Compare
aferrero2707
added a commit
to AliceMCH/QualityControl
that referenced
this pull request
Aug 5, 2025
The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from AliceO2Group#2578
knopers8
pushed a commit
that referenced
this pull request
Aug 8, 2025
* [MCH] added averaging over SOLAR links Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements * [MCH] added helper functions for handling SOLAR indexes and histogram decorations * [MCH] updated Pedestals Task histogram decorations The changes are needed to sync with the updated helper code * [MCH] added plots as function of SOLAR links The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from #2578 * [MCH] added checkers for SOLAR-based plots The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task. * [MCH] added aggregator task for DE and SOLAR qualities The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking. * [MCH] fixed clang errors * [MCH] added review suggestions
knopers8
pushed a commit
that referenced
this pull request
Aug 11, 2025
* [MCH] added averaging over SOLAR links Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements * [MCH] added helper functions for handling SOLAR indexes and histogram decorations * [MCH] updated Pedestals Task histogram decorations The changes are needed to sync with the updated helper code * [MCH] added plots as function of SOLAR links The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from #2578 * [MCH] added checkers for SOLAR-based plots The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task. * [MCH] added aggregator task for DE and SOLAR qualities The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking. * [MCH] fixed clang errors * [MCH] added review suggestions
Barthelemy
pushed a commit
that referenced
this pull request
Aug 11, 2025
* [MCH] added averaging over SOLAR links Added computation of average quantities over SOLAR links, similar to what is already implemented for the averaging over Detection Elements * [MCH] added helper functions for handling SOLAR indexes and histogram decorations * [MCH] updated Pedestals Task histogram decorations The changes are needed to sync with the updated helper code * [MCH] added plots as function of SOLAR links The commit adds plots with quantities averaged over SOLAR links, similar to the existing ones as function of Detection Elements. It also adds comparisons of DE and SOLAR plots with reference ones, using the code from #2578 * [MCH] added checkers for SOLAR-based plots The code that checks the SOLAR-based plots is similar to that for the DE-based ones. The checkers also fill a summary plot with the overall quality of each SOLAR link, which are then aggregared by a dedicated task. * [MCH] added aggregator task for DE and SOLAR qualities The task combines the summary qualities for each DE and SOLAR link, and uploads a CCDB object with the list of bad DE and SOLAR IDs. The CCDB objects will be used to automatically reconfigure the bad links during the data taking. * [MCH] fixed clang errors * [MCH] added review suggestions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes in the PR allow to use the ReferenceComparatorTask as a base class, providing access to the reference comparison plots for the derived classes.
A typical use case would be a post-processing task that produces some derived plots, which also need to compared with some reference.
With the current production code, this involves adding a dedicated ReferenceComparatorTask in the workflow, that fetches the plots from the post-processing task from the QCDB.
This commit allows to solve the above problem by deriving the post-processing task from the ReferenceComparatorTask, such that the reference comparison plots can be filled directly by the derived task, without going through the QCDB.
This is achieved with the following two changes:
update()methodThe PR also adds to the ReferenceUtils.h header file the functions that allow to access the current, reference and ratio plots from the TCanvas created by the ReferenceComparartorPlot class.
This is for example needed to allow custom checkers to access the output of the reference comparison.